Repository navigation
fix(runtime): release native objects on the runtime's thread - #560
Draft
edusperoni wants to merge 3 commits into
Draft
edusperoni wants to merge 3 commits into
edusperoni wants to merge 3 commits into
Conversation
JS can run on any thread that holds the isolate's Locker, for example a JS function dispatched to a background queue. A garbage collection there runs the wrappers' finalizers on that thread, and they released the native objects inline, so a view, a CA transaction or a block capturing one could be deallocated off the main thread and crash. Off the runtime's thread, ObjectManager now hands these releases ([object release] and Block_release, from finalizers and from __releaseNativeCounterpart) to the runtime's run loop. JS-side disposal is unchanged and still immediate. The hand-over is a CFRunLoop block holding a per-isolate queue, not an event loop task, so it still runs after the event loop stops taking work, and teardown drains inline. A new wrapper for an object whose release is still waiting takes that release over as its own retain instead of retaining again, so the retain-count thresholds that protect JS-extended instances stay right; if native code retained the object meanwhile, the new wrapper is protected on the spot. Collection adapters still release inline: their -dealloc retires the wrapper. A worker's runtime thread is the worker's own thread, so objects released by a worker isolate are still released there.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
…read Deferring every off-thread release moved all of a background queue's deallocations onto the main thread, including objects that never belonged there. Wrappers now record whether the object first reached JS on its runtime's thread (a thread-local marker the runtime sets where it binds its event loop, compared with the current isolate when the wrapper is built), and only those releases are handed over. Objects first wrapped on another thread are released where the collection runs, as before. The queue is now plain per-isolate containers: finalizers, __releaseNativeCounterpart, wrapper creation, the drain and teardown all hold the isolate's Locker, so it needs no mutex or atomics. It drains in one event loop task per batch instead of a CFRunLoop block; teardown, which runs after the loop stops, drains it inline before and after the disposal walk.
A single thread-local marker named one isolate per thread, so a second runtime started on the same thread replaced it, and either one's teardown cleared it for both. Each runtime now records its thread where it binds its event loop, and the check compares it with the calling thread through the isolate's runtime.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Current behavior
JS can run on any thread that holds the isolate's
Locker: a JS function passed todispatch_asyncon a background queue runs there, for example. A garbage collection triggered on that thread runs the wrappers' finalizers on it too, and they release the native objects inline. A view, aCATransaction, or a block capturing one can therefore be deallocated off the main thread, which UIKit and Core Animation do not tolerate and which can crash the app.__releaseNativeCounterpartcalled from such a thread has the same problem.New behavior
Which releases move. A native release moves to the runtime's thread when two things hold:
ObjCDataWrapperandBlockWrapperrecord the second at construction. Each runtime records its own thread (pthread_self(), next to its run loop) where it binds its event loop, and the check compares it with the calling thread through the isolate's runtime. Several runtimes can share a thread.Which stay put.
-deallocretires the wrapper.How the handover works.
[object release]orBlock_release, from a finalizer or__releaseNativeCounterpart) goes into a per-isolate queue, drained by one event loop task per batch.Instancesentry erased and the internal field cleared at once.__releaseNativeCounterpart, wrapper creation, the drain task and teardown all hold the isolate'sLocker. The only lock left is the event loop's own, once per batch.Re-wrapping a queued object. A new wrapper for an object whose release is still queued takes that release over as its own retain instead of retaining again. That keeps exact the retain counts the JS-extended classes'
retain/releaseswizzle reads. If native code retained the object in the meantime, the new wrapper is protected on the spot, since the swizzle had no wrapper to protect then.Approximations.
On the runtime's thread nothing changes except a thread-id compare per ObjC or block wrapper creation and disposal.
Documented in
docs/knowledge/v8-resurrecting-finalizers.md("Which thread releases the native object").Testing
New specs in
GCFinalizerTests.js. Both use a JS-extendedTNSApiinstance owned only by its wrapper, which recordsNSThread.isMainThreadfrommethodCalledInDealloc;__collect()runs in adispatch_asyncblock on a background queue.Expected $[0] = false to equal true).Full suite, 0 failures (1869 specs).